Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DynamicNBEATs model #1191

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

DynamicNBEATs model #1191

wants to merge 6 commits into from

Conversation

tblume1992
Copy link

By default NBEATs only utilizes a polynomial basis for the trend component. This PR adds support for several different basis functions for the trend and adds a handler for these functions in a new 'DynamicNBEATs' method.

@tblume1992
Added DynamicNBEATs notebook
5993729 
Supports several basis functions for the trend and adds a handler for these functions
@review-notebook-app Review Notebook App
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@CLAassistant
Copy link

CLAassistant commented Nov 3, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@elephaint
Copy link
Contributor

elephaint commented Nov 14, 2024 •
edited
Loading

Hi, thanks for your contribution, this is really cool! Two comments:

  1. Could you please follow the contributing guidelines? Specifically, we need you to build the library so that the correct python files are generated.
  2. Why not adjust NBEATs to support different basis by adding a parameter that allows to choose the basis? I'd assume that would save a lot of code.

@tblume1992
Copy link
Author

  1. hey @elephaint I was chatting with @marcopeix about this awhile back. Unfortunately I don't have time for a PR but he wanted me to be the author of it. All the code was just in a notebook I threw into the PR.

  2. You can definitely do that and that is basically what I did. 95% of this code is just the base NBEATs code. I think though that this change would alter the 'auto' model pretty significantly since we would want to search for the basis functions AND the number of functions. So we may want to think of it as a different model.

@marcopeix marcopeix requested a review from elephaint December 13, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elephaint elephaint Awaiting requested review from elephaint

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tblume1992 @CLAassistant @elephaint @marcopeix